🤖 AI-assisted review

This review has been superseded by the "Changes requested" review below. See that review for the full findings and the suggested fix.

🤖 AI-assisted review

The intent of this PR is correct, and it genuinely fixes clicks on nested label text and implicit-wrap labels that the previous code missed. However, one change widens the suppression too far and introduces a regression that silently drops real user clicks from the recording. Because this is a recorder — a missed step produces a quietly incomplete recording that only fails later on replay — I'm requesting changes on that one item before merge. The version bumps and OUTPUT-EXAMPLE.md were verified and look good.

Blocking

1. 🔴 src/pages/Content/modules/collectEvents.js:202 — Clicks on links/buttons inside a checkbox label are dropped (regression)

Switching from nodeName === 'label' to tgt.closest('label') makes the early-return fire for any click inside a label that contains or references a checkbox/radio — including clicks on interactive descendants such as <a> and <button>. The browser does not forward those clicks to the labeled control (per the HTML standard, a label's activation behavior does nothing for clicks targeted at interactive content descendants), so no change event fires and the click is a distinct user action — but it gets silently dropped from the recording.

A common trigger is the consent pattern:

<label><input type="checkbox"> I agree to the <a href="/terms">terms</a></label>

Clicking "terms" navigates but is no longer recorded. The previous code did not have this issue (it only matched when the click target was the <label> element itself), so this is a regression — while the rest of the change is a genuine improvement. Behavior across cases:

Suggested fix — only suppress when the click is actually forwarded to the labeled control (also makes the for lookup work inside shadow DOM, addressing item 2):

const parentLabel = tgt.closest && tgt.closest('label');
if (parentLabel) {
  // Only suppress when the click is actually forwarded to the labeled control.
  // Clicks on interactive descendants (links, buttons, other fields) are NOT
  // forwarded by the browser and must still be recorded.
  const interactiveHit = tgt.closest('a, button, input, select, textarea');
  const forwardsToControl = !interactiveHit || !parentLabel.contains(interactiveHit);
  if (forwardsToControl) {
    const forAttr = parentLabel.getAttribute('for');
    if (forAttr) {
      const root = tgt.getRootNode();
      const lookup = root && root.getElementById ? root : document; // shadow-DOM safe
      const forEl = lookup.getElementById(forAttr);
      if (forEl && forEl.nodeName.toLowerCase() === 'input' &&
        (forEl.type === 'checkbox' || forEl.type === 'radio')) {
        return;
      }
    }
    if (parentLabel.querySelector('input[type="checkbox"], input[type="radio"]')) {
      return;
    }
  }
}

This keeps the intended suppression for label text / nested non-interactive elements and only stops dropping clicks on interactive descendants. Since test-build/ and test-build-firefox/ are committed, please run ./buildit.sh after the source change so the bundles stay in sync.

Nice to have

2. 🟢 src/pages/Content/modules/collectEvents.js:206 — Shadow DOM for-association uses global document

document.getElementById(forAttr) cannot resolve a checkbox that lives inside a shadow root, so a redundant label click in shadow DOM is not suppressed. Using tgt.getRootNode().getElementById(...) (folded into the fix above) handles both the regular page and shadow DOM. Minor and pre-existing.

Notes on the rest of the PR: version bumps are consistent (package.json, manifest-firefox.json, and both build manifests are 1.2.1; the Chrome manifest.json is intentionally version-less and injected at build time). OUTPUT-EXAMPLE.md is valid and the #outer-iframe >>> #inner-iframe format matches how frame paths are built (outermost to innermost). Both regenerated content-script bundles are identical and reflect the source change.

🤖 AI-assisted review

Verified the follow-up commit (a2d258f): the label/checkbox click handler now suppresses only the clicks the browser actually forwards to the labeled control, and records clicks on interactive descendants (links, buttons, selects, text fields). Re-tested the shipped logic across 14 label configurations — for- and wrap-association, nested spans, links, buttons, selects, radios, and shadow DOM — and all behave correctly, so the earlier regression (dropping clicks on links/buttons inside a checkbox label) is resolved. Version is consistent at 1.2.2 and both content-script bundles are correctly regenerated. CI is green.

Non-blocking nit: the PR title still says "v1.2.1" — consider updating it to v1.2.2 for accurate release notes.